-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RLlib] Add necessary fields to Base Envs, and BaseEnv wrapper classes #20832
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this.
do observation_space(self) and action_space(self) work for all the other wrapper types?
They don't work yet, I am adding them in, which is why I made this a draft PR 😁 |
ok, ping after you are done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still looks very nice. like the dict space design for multi-agent env.
just have one question, is it really worth it to have all the sub_environments() function take an additional as_dict parameter?
why not always return a dict?
it's not gonna make big difference when you use it, since you can simply .values() the returned dict.
8e0ed14
to
030222f
Compare
looks great to me!! |
a115b2d
to
e4a22f6
Compare
rllib/env/vector_env.py
Outdated
|
||
@staticmethod | ||
def _space_contains(space: gym.Space, x: MultiEnvDict) -> bool: | ||
"""Check if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you complete this docstring?
830c036
to
f858585
Compare
This is an initial commit. Adding observation_space
action_space, and a method for retrieving env_ids as
requirements to the BaseEnv Class
Will need to update for all of the other BaseEnv
wrappers
Why are these changes needed?
Base Envs are missing attributes that are necessary for validating them.
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.